Skip to content

feature(cairo): Support Span<T> destructuring via fixed-size array pa…#9823

Open
ilyalesokhin-starkware wants to merge 1 commit intomainfrom
ilya/span_unpack
Open

feature(cairo): Support Span<T> destructuring via fixed-size array pa…#9823
ilyalesokhin-starkware wants to merge 1 commit intomainfrom
ilya/span_unpack

Conversation

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor

@ilyalesokhin-starkware ilyalesokhin-starkware commented Apr 6, 2026

…tterns

Add support for let [a, b] = span else { ... } syntax by introducing a SliceDestructure flow control node that calls tuple_from_span to convert Span to [T; N] at runtime.


Note

Medium Risk
Touches semantic type-checking and match/let lowering by adding a new flow-control node and runtime conversion path; bugs here could affect pattern matching correctness and generated code for matches involving Span<T> and fixed-size array patterns.

Overview
Adds support for matching/destructuring Span<T> with fixed-size array patterns (e.g. let [a, b] = span else { ... } and match span { [a,b] => ... }).

Lowering now builds a size-dispatch chain for Span<T> patterns via a new FlowControlNode::SliceDestructure, which calls corelib tuple_from_span (with FixedSizedArrayInfoImpl) and handles success/failure branching. Semantic pattern computation is updated to accept fixed-size array patterns on Span types, and diagnostics/tests are extended to cover the new behavior.

Reviewed by Cursor Bugbot for commit dbd4689. Bugbot is set up for automated code reviews on this repo. Configure here.

@reviewable-StarkWare
Copy link
Copy Markdown

This change is Reviewable

Comment thread crates/cairo-lang-semantic/src/expr/compute.rs Outdated
@ilyalesokhin-starkware ilyalesokhin-starkware marked this pull request as draft April 6, 2026 13:55
@ilyalesokhin-starkware ilyalesokhin-starkware changed the base branch from main to graphite-base/9823 April 9, 2026 08:38
@ilyalesokhin-starkware ilyalesokhin-starkware changed the base branch from graphite-base/9823 to ilya/span_pattern_semantic April 9, 2026 08:38
Copy link
Copy Markdown
Contributor Author

ilyalesokhin-starkware commented Apr 9, 2026

@ilyalesokhin-starkware ilyalesokhin-starkware changed the base branch from ilya/span_pattern_semantic to graphite-base/9823 April 9, 2026 13:19
@ilyalesokhin-starkware ilyalesokhin-starkware changed the base branch from graphite-base/9823 to main April 12, 2026 06:22
Comment thread crates/cairo-lang-semantic/src/expr/compute.rs Outdated
Copy link
Copy Markdown
Contributor Author

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ilyalesokhin-starkware resolved 1 discussion.
Reviewable status: 0 of 11 files reviewed, 1 unresolved discussion (waiting on orizi).

Copy link
Copy Markdown
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orizi made 2 comments.
Reviewable status: 0 of 11 files reviewed, 3 unresolved discussions (waiting on ilyalesokhin-starkware).


corelib/src/test/language_features/match_test.cairo line 26 at r3 (raw file):

            assert_eq!(b, @20);
            assert_eq!(c, @30);
        },

Suggestion:

        [a, b, c] => {
            assert_eq!((*a, *b, *c), (10, 20, 30));
        },

crates/cairo-lang-lowering/src/lower/flow_control/test_data/match line 2603 at r3 (raw file):

    match s {
        [a, b] => *a + *b,
        [a, b] => *a + *b,

add also:

Suggestion:

    match s {
        [a, b] | [a, b, _] => *a + *b,

@ilyalesokhin-starkware ilyalesokhin-starkware force-pushed the ilya/span_unpack branch 3 times, most recently from 9df5995 to 0aa3fa0 Compare April 14, 2026 08:41
@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 465 at r5 (raw file):

                        group.filter.add(idx);
                        group.patterns.push(None);
                    }

Not sure if I should do that like in create_node_for_enum, or just use break here.

Code quote:

                    opt_wildcard_idx = Some(idx);
                    for group in size_groups.values_mut() {
                        group.filter.add(idx);
                        group.patterns.push(None);
                    }

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

corelib/src/test/language_features/match_test.cairo line 26 at r3 (raw file):

            assert_eq!(b, @20);
            assert_eq!(c, @30);
        },

Done.

Copy link
Copy Markdown
Contributor Author

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ilyalesokhin-starkware made 1 comment and resolved 1 discussion.
Reviewable status: 0 of 11 files reviewed, 3 unresolved discussions (waiting on orizi).


crates/cairo-lang-lowering/src/lower/flow_control/test_data/match line 2603 at r3 (raw file):

Previously, orizi wrote…

add also:

Done.

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 465 at r5 (raw file):

Previously, ilyalesokhin-starkware wrote…

Not sure if I should do that like in create_node_for_enum, or just use break here.

see r5 <-> r6.

@ilyalesokhin-starkware ilyalesokhin-starkware marked this pull request as ready for review April 14, 2026 10:56
@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 503 at r10 (raw file):

Previously, orizi wrote…

just the goto at the end the arm to the continuation of the flow.

We don't generated code for the unreacheable arms, just diagnostics

Copy link
Copy Markdown
Contributor Author

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ilyalesokhin-starkware made 2 comments.
Reviewable status: 8 of 11 files reviewed, 4 unresolved discussions (waiting on orizi).


crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 354 at r10 (raw file):

            elem_ty,
        );
    }

Done.


crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 443 at r10 (raw file):

    let snapshot_member_id = members.iter().next().unwrap().1.id;
    let snapshot_array_ty = members.iter().next().unwrap().1.ty;
    let snapshot_array_var = graph.new_var(snapshot_array_ty, location);

Done.

Copy link
Copy Markdown
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orizi reviewed 3 files and all commit messages, made 1 comment, and resolved 3 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on ilyalesokhin-starkware).


crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 411 at r9 (raw file):

Previously, ilyalesokhin-starkware wrote…

"can open the type"?

i think i didn't understand the original question.

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 411 at r9 (raw file):

Previously, orizi wrote…

i think i didn't understand the original question.

I'm wondering if we are going to have a

 Span { snapshot: ... }

pattern that is not a catch all pattern.

In that case, I can't just break after reaching that pattern.

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 411 at r9 (raw file):

Previously, ilyalesokhin-starkware wrote…

I'm wondering if we are going to have a

 Span { snapshot: ... }

pattern that is not a catch all pattern.

In that case, I can't just break after reaching that pattern.

fox example:

if
Span { snapshot: inner } if inner.len() == 5 was supported, then the break is incorrect.

but I think the code to support it would be much more complicated.

Copy link
Copy Markdown
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orizi made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on ilyalesokhin-starkware).


crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 411 at r9 (raw file):

Previously, ilyalesokhin-starkware wrote…

fox example:

if
Span { snapshot: inner } if inner.len() == 5 was supported, then the break is incorrect.

but I think the code to support it would be much more complicated.

we must cover all patterns - either by explicit matching - or a final _ - same as rust

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 411 at r9 (raw file):

Previously, orizi wrote…

we must cover all patterns - either by explicit matching - or a final _ - same as rust

The question is not whether we cover everything.

It is whether the code can assume that a struct pattern on a slice is a catch-all pattern.

Copy link
Copy Markdown
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orizi made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on ilyalesokhin-starkware).


crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 411 at r9 (raw file):

Previously, ilyalesokhin-starkware wrote…

The question is not whether we cover everything.

It is whether the code can assume that a struct pattern on a slice is a catch-all pattern.

yes - match guards would change that - but since we don't have them currently - it is fine.

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 411 at r9 (raw file):

Previously, orizi wrote…

yes - match guards would change that - but since we don't have them currently - it is fine.

Done.

Copy link
Copy Markdown
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@orizi reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on ilyalesokhin-starkware).

Copy link
Copy Markdown
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orizi reviewed all commit messages and made 1 comment.
Reviewable status: 10 of 11 files reviewed, 1 unresolved discussion (waiting on ilyalesokhin-starkware).


crates/cairo-lang-lowering/src/lower/flow_control/test_data/match line 3077 at r13 (raw file):


//! > lowering_diagnostics
error[E3004]: Match is non-exhaustive: `(Span { snapshot: _ }, _)` not covered.

this seems like a bug - right?

@ilyalesokhin-starkware ilyalesokhin-starkware marked this pull request as draft April 19, 2026 11:42
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9be728f. Configure here.

}
// A catch-all struct pattern makes every subsequent arm unreachable, so we stop
// adding to `size_groups`. The match-completeness pass will flag the dead arms.
break;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Struct pattern break drops subsequent reachable arms in tuples

High Severity

The break after processing a Span { snapshot: ... } struct pattern in create_slice_destructure_chain incorrectly drops all subsequent patterns. While Span { snapshot: inner } is a catch-all for the span dimension, it is NOT a catch-all when nested inside a tuple match — subsequent arms with different patterns on other tuple elements remain reachable. This causes valid exhaustive matches like match (s, val) { ..., (Span { snapshot: inner }, 5) => ..., _ => 0 } to produce a false Match is non-exhaustive compilation error, because the _ => 0 arm is never added to any filter. The test confirms this via the Missing node (node 5) and the spurious error diagnostic.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9be728f. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

crates/cairo-lang-lowering/src/lower/flow_control/test_data/match line 3077 at r13 (raw file):

Previously, orizi wrote…

this seems like a bug - right?

yes, it seems that the approch of breaking after a wildcard does not work.

@ilyalesokhin-starkware ilyalesokhin-starkware force-pushed the ilya/span_unpack branch 2 times, most recently from ad2e0ae to 3d1cf88 Compare April 20, 2026 09:58
Copy link
Copy Markdown
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orizi made 2 comments and resolved 1 discussion.
Reviewable status: 9 of 11 files reviewed, 3 unresolved discussions (waiting on ilyalesokhin-starkware).


crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 467 at r15 (raw file):

                    inner_patterns: vec![None; fallback_group.inner_patterns.len()],
                    inner_bindings: fallback_group.inner_bindings.clone(),
                });

Suggestion:

                let group = size_groups.entry(n).or_insert_with(|| fallback_group.clone());

crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 549 at r15 (raw file):

    // callback. The path string is `"_"` because the match has no `[..]` syntax to name the
    // fallback case.
    let mut failure_node = {

Suggestion:

    let mut current_node = {

}
// A catch-all struct pattern makes every subsequent arm unreachable, so we stop
// adding to `size_groups`. The match-completeness pass will flag the dead arms.
break;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 467 at r15 (raw file):

                    inner_patterns: vec![None; fallback_group.inner_patterns.len()],
                    inner_bindings: fallback_group.inner_bindings.clone(),
                });

Done.

@ilyalesokhin-starkware
Copy link
Copy Markdown
Contributor Author

crates/cairo-lang-lowering/src/lower/flow_control/create_graph/patterns.rs line 549 at r15 (raw file):

    // callback. The path string is `"_"` because the match has no `[..]` syntax to name the
    // fallback case.
    let mut failure_node = {

Done.

@ilyalesokhin-starkware ilyalesokhin-starkware marked this pull request as ready for review April 20, 2026 11:32
Copy link
Copy Markdown
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@orizi reviewed 3 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on ilyalesokhin-starkware).

Copy link
Copy Markdown
Contributor Author

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ilyalesokhin-starkware resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on ilyalesokhin-starkware).

Copy link
Copy Markdown
Contributor Author

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ilyalesokhin-starkware reviewed 11 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on ilyalesokhin-starkware).

…tterns

Add support for `let [a, b] = span else { ... }` syntax by introducing
a SliceDestructure flow control node that calls tuple_from_span to
convert Span<T> to [T; N] at runtime.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants